Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/stim grammar #45

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Open

Feature/stim grammar #45

wants to merge 17 commits into from

Conversation

maartenuni
Copy link
Contributor

This is a feature reworks the way how stimuli are written for the jspsych-spr-reading. Using the javascript nearley parser and moo lexer frameworks it is possible to create a stimulus where you can be more confident how the stimulus will finaly appear on screen. The former stimulus was removing and replacing characters from the input along a number of dimension which made it hard to see what it was doing.
Now there a grammar that dictates how stimuli should be written see plugins/grammar.ne which is converted to plugins/grammar.js. This is all packaged using esbuild to the dist directory. you can call ./build.sh to recreate grammar.js and populate dist. with a packaged version of the plugin.

It is now easier to create a number of bold/italic words in a row as you can put multiple words within tags, whereas before you had to tag each individual word. The grammar is much stricter than a html parser, as groups need to be closed e.g see the readme for more details

@maartenuni maartenuni requested a review from bbonf January 17, 2025 11:06
Copy link
Contributor

@bbonf bbonf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the new approach!

Previously, when the spr was presenting multiple words as one group/simultaneous, you
needed to separate them by inserting e.g. a `/`, as of now, you have to
specifically create groups yourself and grouping of words is always on. On
the. There are two kinds of groups `{{A group of words that is not recorded}}`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the ... (missing word?)

needed to separate them by inserting e.g. a `/`, as of now, you have to
specifically create groups yourself and grouping of words is always on. On
the. There are two kinds of groups `{{A group of words that is not recorded}}`
`{{#A group of words that is NOT recorded.}}`. Recorded means that the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove NOT?

output. You should not put word letters etc outside of a group.

All the words of your spr need to be enclosed in **{{**double curly braces**}}**
so the phrase "double curly braces" is presented together. If you want the to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wan the ... to (missing word)

the stimulus into parts and to remove the `/` in the case described here.
In this example the spaces are embedded inside the groups, both methods are
fine, but the author finds the first method better readable. If you put
none white space characters outside of a group it will be considered a syntax error.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure I understand, maybe add an example?

from the stimulus. So you should take in mind how the stimulus would appear
after the grouping string is removed.
However, the syntax for this is more strict than HTML, forgetting to close a bold
or italic tag will result in an error. The bold or you'll have to close the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bold or ... you'll (missing word)

advanceWhiteSpace(context, group);
}
else if (type == "Group") {
let parts = group.sentence_parts.parts;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might be easier to follow if you also have an advanceGroup function

}
);

// for (let line = 0; line < lines.length; line++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove commented out code

parser.feed(stimulus)
let tree = parser.results
if (tree.length > 1) {
console.error("The grammar is ambigious for this stimulus");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this should throw an error instead?

console.error("The grammar is ambigious for this stimulus");
}
else if (tree.length == 0) {
console.log("stimulus: \"" + stimulus +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same?

parser.feed(trial.stimulus)
let tree = parser.results
if (tree.length > 1) {
console.error("Oops, grammar is ambiguous.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants